Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

For Discussion: Using tinyvec::ArrayVec for Position without allocating #218

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

For Discussion: Using tinyvec::ArrayVec for Position without allocating #218

wants to merge 1 commit into from

Conversation

phayes
Copy link

@phayes phayes commented Feb 28, 2023 •
edited
Loading

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

This is a PR for discussion. I've moved Position from being a Vec<f64> to being an ArrayVec<f64> with a capacity of 4 elements (x, y, z, t). This means that instead of every point resulting in an allocation, we only allocate to create LineString, Polygon etc.

Preliminary cargo-bench results suggest performance improvements up to 79% in some cases.

If there is agreement that this is a good direction, I'll add some convenience functions like Position::from_vec etc to recover some of the ergonomics of working with Vecs.

Benchmarks:

Benchmarking parse (countries.geojson)
Benchmarking parse (countries.geojson): Warming up for 3.0000 s
Benchmarking parse (countries.geojson): Collecting 100 samples in estimated 5.1934 s (2600 iterations)
Benchmarking parse (countries.geojson): Analyzing
parse (countries.geojson)
                        time:   [1.9881 ms 1.9981 ms 2.0086 ms]
                        change: [-13.706% -13.086% -12.475%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

Benchmarking FeatureReader::features (countries.geojson)
Benchmarking FeatureReader::features (countries.geojson): Warming up for 3.0000 s
Benchmarking FeatureReader::features (countries.geojson): Collecting 100 samples in estimated 5.5955 s (900 iterations)
Benchmarking FeatureReader::features (countries.geojson): Analyzing
FeatureReader::features (countries.geojson)
                        time:   [6.1732 ms 6.1837 ms 6.1956 ms]
                        change: [-6.3597% -6.0745% -5.7713%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  7 (7.00%) high mild
  2 (2.00%) high severe

Benchmarking FeatureReader::deserialize (countries.geojson)
Benchmarking FeatureReader::deserialize (countries.geojson): Warming up for 3.0000 s
Benchmarking FeatureReader::deserialize (countries.geojson): Collecting 100 samples in estimated 5.5427 s (700 iterations)
Benchmarking FeatureReader::deserialize (countries.geojson): Analyzing
FeatureReader::deserialize (countries.geojson)
                        time:   [8.1079 ms 8.1365 ms 8.1734 ms]
                        change: [-2.0054% -1.5319% -0.9778%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

Benchmarking FeatureReader::deserialize to geo-types (countries.geojson)
Benchmarking FeatureReader::deserialize to geo-types (countries.geojson): Warming up for 3.0000 s
Benchmarking FeatureReader::deserialize to geo-types (countries.geojson): Collecting 100 samples in estimated 5.5704 s (700 iterations)
Benchmarking FeatureReader::deserialize to geo-types (countries.geojson): Analyzing
FeatureReader::deserialize to geo-types (countries.geojson)
                        time:   [7.9112 ms 7.9288 ms 7.9473 ms]
                        change: [-2.6196% -2.3486% -2.0713%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

Benchmarking parse (geometry_collection.geojson)
Benchmarking parse (geometry_collection.geojson): Warming up for 3.0000 s
Benchmarking parse (geometry_collection.geojson): Collecting 100 samples in estimated 5.0480 s (293k iterations)
Benchmarking parse (geometry_collection.geojson): Analyzing
parse (geometry_collection.geojson)
                        time:   [17.236 µs 17.274 µs 17.320 µs]
                        change: [-7.0696% -6.7231% -6.3747%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

WARNING: HTML report generation will become a non-default optional feature in Criterion.rs 0.4.0.
This feature is being moved to cargo-criterion (https://github.com/bheisler/cargo-criterion) and will be optional in a future version of Criterion.rs. To silence this warning, either switch to cargo-criterion or enable the 'html_reports' feature in your Cargo.toml.

Gnuplot not found, using plotters backend
Benchmarking serialize geojson::FeatureCollection struct (countries.geojson)
Benchmarking serialize geojson::FeatureCollection struct (countries.geojson): Warming up for 3.0000 s
Benchmarking serialize geojson::FeatureCollection struct (countries.geojson): Collecting 100 samples in estimated 5.0413 s (1700 iterations)
Benchmarking serialize geojson::FeatureCollection struct (countries.geojson): Analyzing
serialize geojson::FeatureCollection struct (countries.geojson)
                        time:   [2.9583 ms 2.9634 ms 2.9690 ms]
                        change: [-2.4235% -1.8933% -1.3582%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) high mild
  4 (4.00%) high severe

Benchmarking serialize custom struct (countries.geojson)
Benchmarking serialize custom struct (countries.geojson): Warming up for 3.0000 s
Benchmarking serialize custom struct (countries.geojson): Collecting 100 samples in estimated 5.1482 s (2300 iterations)
Benchmarking serialize custom struct (countries.geojson): Analyzing
serialize custom struct (countries.geojson)
                        time:   [2.2299 ms 2.2331 ms 2.2364 ms]
                        change: [-2.4677% -2.2513% -2.0310%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

Benchmarking serialize custom struct to geo-types (countries.geojson)
Benchmarking serialize custom struct to geo-types (countries.geojson): Warming up for 3.0000 s
Benchmarking serialize custom struct to geo-types (countries.geojson): Collecting 100 samples in estimated 5.0427 s (2200 iterations)
Benchmarking serialize custom struct to geo-types (countries.geojson): Analyzing
serialize custom struct to geo-types (countries.geojson)
                        time:   [2.2959 ms 2.3017 ms 2.3081 ms]
                        change: [-11.260% -11.003% -10.713%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  7 (7.00%) high mild
  2 (2.00%) high severe

WARNING: HTML report generation will become a non-default optional feature in Criterion.rs 0.4.0.
This feature is being moved to cargo-criterion (https://github.com/bheisler/cargo-criterion) and will be optional in a future version of Criterion.rs. To silence this warning, either switch to cargo-criterion or enable the 'html_reports' feature in your Cargo.toml.

Gnuplot not found, using plotters backend
Benchmarking quick_collection
Benchmarking quick_collection: Warming up for 3.0000 s
Benchmarking quick_collection: Collecting 100 samples in estimated 5.1283 s (71k iterations)
Benchmarking quick_collection: Analyzing
quick_collection        time:   [72.840 µs 73.421 µs 74.013 µs]
                        change: [-79.655% -79.490% -79.317%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

@phayes phayes changed the title For Discussion: Using tinyvec::ArrayVec for position to store positions on the stack For Discussion: Using tinyvec::ArrayVec for position to store positions without allocating Feb 28, 2023
@phayes phayes changed the title For Discussion: Using tinyvec::ArrayVec for position to store positions without allocating For Discussion: Using tinyvec::ArrayVec for position without allocating Feb 28, 2023
@phayes phayes changed the title For Discussion: Using tinyvec::ArrayVec for position without allocating For Discussion: Using tinyvec::ArrayVec for Position without allocating Feb 28, 2023
@michaelkirk
Copy link
Member

See also: #88, and in particular, my comment

@michaelkirk michaelkirk mentioned this pull request Apr 6, 2023
Draft
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@phayes @michaelkirk